Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(openapi): generate merged spec #497

Conversation

jvallesm
Copy link
Collaborator

Because

  • SDKs like tryAGI need a single
    OpenAPI definition.
  • Merging our definitions would make sense as the public API is exposed by
    a single service (api-gateway). Headers represent that service's
    interface rather than the backend ones.

This commit

  • Generates a merged OpenAPI definition.
    • For now individual definitions are respected and used to feed openapi.instill.tech.
      This adds some duplication in the configuration (/<service>/openapi.proto.templ and /openapiv2/conf.proto).
      We need to decide if we want to use the merged OpenAPI instead (we'll lose the service as a category and the endpoints will be only grouped by tags.
    • Tags are respected as much as possible, just modified to avoid
      collisions.
    • Depending on whether we want to transition to this single OpenAPI for
      readme.com, we might consider group endpoints by service or
      merge some tags ([User, Organization] -> Namespace).

@jvallesm jvallesm self-assigned this Oct 24, 2024
Copy link

linear bot commented Oct 24, 2024

Copy link

github-actions bot commented Oct 24, 2024

The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedOct 28, 2024, 10:08 AM

@jvallesm jvallesm force-pushed the jvalles/ins-6510-merge-all-public-endpoints-into-single-openapi-file branch 2 times, most recently from 13ac045 to 85eb0ab Compare October 24, 2024 08:53
@jvallesm jvallesm marked this pull request as ready for review October 24, 2024 09:28
@jvallesm jvallesm force-pushed the jvalles/ins-6510-merge-all-public-endpoints-into-single-openapi-file branch 6 times, most recently from 68a8f20 to 2c957ee Compare October 28, 2024 07:42
It's an issue that doesn't happen with the newer versions of
`grpc-gateway`, so the generated file doesn't change with or without
this command.
Using a nested path parameter produced lint errors in the OpenAPI spec:
- Reference wasn't detected in the request object
- `/v1beta/namespaces/{}/connections[/{}]` wasn't a unique path.
@jvallesm jvallesm force-pushed the jvalles/ins-6510-merge-all-public-endpoints-into-single-openapi-file branch from 2c957ee to d33c50d Compare October 28, 2024 08:30
@jvallesm jvallesm force-pushed the jvalles/ins-6510-merge-all-public-endpoints-into-single-openapi-file branch from d33c50d to 9299bbc Compare October 28, 2024 08:33
@jvallesm jvallesm merged commit 9d69b5c into main Oct 28, 2024
12 checks passed
@jvallesm jvallesm deleted the jvalles/ins-6510-merge-all-public-endpoints-into-single-openapi-file branch October 28, 2024 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants